Skip to content

feat: wire boilerplate to crudauth follow-ups - #292

Merged
igorbenav merged 3 commits into
benavlabs:mainfrom
emiliano-go:feat/crudauth-followup-integration
Sep 19, 2026
Merged

igorbenav merged 3 commits into
benavlabs:mainfrom
emiliano-go:feat/crudauth-followup-integration

Conversation

@emiliano-go

Copy link
Copy Markdown
Collaborator

Wires the boilerplate to the follow-up crudauth APIs and removes the duplicated application auth infrastructure.

Changes:

  • configure the shared password policy;
  • apply crudauth dynamic rate limiting with request-level principal resolution;
  • inject application Redis clients into crudauth;
  • use crudauth's configurable OAuth router with JSON responses and browser-bound state;
  • preserve the existing auth URL contract where possible;
  • update configuration and documentation.

This PR must remain blocked until the following crudauth PRs are merged and released:

The dependency is set to crudauth>=0.7.0,<0.8.0 because that release is required for this branch.

Rebuild the crudauth integration on top of current main and crudauth 0.7.0:

- configure the shared password policy and drive the signup schema from the
  same PASSWORD_* settings so both enforce the same rules;
- use crudauth's built-in OAuth router (configurable paths, JSON responses,
  browser-bound state) instead of the hand-rolled Google routes;
- apply crudauth's per-request, tier/path-aware rate limiting with a
  router-level dependency, replacing the in-tree middleware/provider;
- inject the shared Redis clients into crudauth and the cache backend;
- delete the now-dead infrastructure/rate_limit package, its tests, and the
  vestigial RATE_LIMITER_BACKEND / FAIL_OPEN / MEMCACHED settings;
- update tests and docs.
@emiliano-go

Copy link
Copy Markdown
Collaborator Author

Rebuilt this branch on top of current main and crudauth 0.7.0; it was 55+ commits behind, and the crudauth PRs it depended on (#22 to #28) are now merged and released.

New commits:

  • bump crudauth[all] to >=0.7.0,<0.8.0; uv.lock picks up cryptography from the all extra;
  • configure the password policy once via PASSWORD_* and drive the signup schema from the same settings, so the schema and the crudauth policy enforce identical rules;
  • run OAuth on crudauth's configurable router (JSON responses, browser-bound state, same /oauth/{provider} paths), and remove the hand-rolled Google routes plus auth/oauth.py;
  • move rate limiting to crudauth's per-request, tier and path aware limiter via a router-level dependency;
  • add infrastructure/redis.py with the shared Redis clients injected into crudauth and the cache backend;
  • delete the dead infrastructure/rate_limit package and its tests, along with the vestigial RATE_LIMITER_BACKEND, RATE_LIMITER_FAIL_OPEN and RATE_LIMITER_MEMCACHED_* settings;
  • keep the upstream session storage (SESSION_REDIS_URL), logout-all, and read-endpoint auth changes;
  • update tests and docs.

Validation: ruff and mypy clean; 284 tests pass; docs build clean.

@emiliano-go
emiliano-go force-pushed the feat/crudauth-followup-integration branch from 1a92ec9 to 5c555da Compare September 18, 2026 03:43
… rate-limit budget, take the password field from the policy, and test all of it
… logins, and hash passwords off the event loop
@igorbenav

Copy link
Copy Markdown
Collaborator

fixed some things:

OAuth, three problems:

  • The callback URI sent to Google lost its /api/v1/auth segment. crudauth builds it as redirect_base_url + prefix + callback_path, and with OAUTH_REDIRECT_BASE_URL=http://localhost:8000 that came out as http://localhost:8000/oauth/callback/google, while the route lives at /api/v1/auth/oauth/callback/{provider}. Every deployment would have hit redirect_uri_mismatch. Putting /api/v1/auth into redirect_base_url isn't the fix, because crudauth also uses that value for the post-login default and the error redirect, which would then point at a 404. So the router is now mounted at the app root with prefix="/api/v1/auth/oauth". The env var keeps meaning "the origin", and the URI matches what's already registered in the Google console.
  • Every first-time Google sign-in returned 500: __init__() missing 1 required positional argument: 'name'. User is a dataclass with a required name, and the deleted auth/oauth.py supplied it through new_user_fields. That's restored.
  • The callback ran in JSON mode, but Google sends the browser there, so users would have landed on a raw JSON page. It's in redirect mode now, which was main's default. The catch is that authorize redirects too, instead of returning {"url"}; that's in the migration notes.

Rate limiting, two problems:

  • crudauth keys a budget by action and identity, not path, so every route shared one counter while the limit changed per path. Three calls to /tiers/ and the first call to /rate-limits/ got a 429. The key now includes the path, which matches the old per-path behavior and what the docs promised.
  • The tier lookup opened local_session() directly, so it skipped dependency injection and, under tests, went to the configured Postgres instead of the test database. No test caught it, because auth_client overrides get_current_user and crudauth never sees a principal there, so the tier branch never ran. It now reads through the app's own DB dependency.

A few smaller changes:

  • RATE_LIMITER_BACKEND (redis or memory) is back as its own setting instead of following SESSION_BACKEND. memcached fails at startup rather than silently moving counters into one process.
  • UserCreate.password now comes from password_policy.body_field(), so the schema can't drift from the policy crudauth enforces. It also means a rejected password gets a 422 naming each requirement instead of the generic validation message.
  • /login finishes through the session transport's complete_login and refuses cross-site requests (it accepted them before). Signup and the admin form now hash off the event loop.

Tests: the branch had deleted the OAuth and limiter suites without adding replacements, so I added 8 OAuth tests against the app as mounted, 4 end-to-end rate-limit tests including the tier path, wiring tests, and signup and login coverage. That's 307 passing, and nine mutations of the fixes are each caught. Because pytest gives each test its own event loop and the Redis clients break across loops, the production path was also run once in a single process against real Postgres 16 and Redis 7. Login, per-path limits, the policy, and a full Google callback all behaved, and the counters landed in Redis under the per-path keys.

@igorbenav
igorbenav merged commit 1bbca31 into benavlabs:main Sep 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants